feat(chat): add compact tool UI mode for executed tool blocks (#322) - #327
feat(chat): add compact tool UI mode for executed tool blocks (#322)#327proyectoauraorg wants to merge 5 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Walkthrough(Note: the hidden artifact above is machine-parsed; do not edit.)
|
| Layer / File(s) | Summary |
|---|---|
Schema, types, provider and context state packages/types/src/global-settings.ts, packages/types/src/vscode-extension-host.ts, webview-ui/src/context/ExtensionStateContext.tsx, src/core/webview/ClineProvider.ts |
Adds compactToolUI to globalSettingsSchema and ExtensionState, initializes compactToolUI: false in ExtensionStateContext provider, and wires compactToolUI into ClineProvider state posted to the webview. |
Settings UI and state wiring webview-ui/src/components/settings/ContextManagementSettings.tsx, webview-ui/src/components/settings/SettingsView.tsx |
ContextManagementSettings accepts a compactToolUI prop, renders a checkbox that calls setCachedStateField("compactToolUI", ...); SettingsView reads compactToolUI from cachedState, includes it in update payloads, and forwards it to ContextManagementSettings. |
Chat row compact rendering webview-ui/src/components/chat/ChatRow.tsx |
ChatRow imports ChevronRight, reads compactToolUI from extension state, and returns an early single-line clickable summary (chevron + tool label) for executed say tool rows when compact mode is enabled and the row is collapsed. |
Tests and internationalization webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx, webview-ui/src/i18n/locales/*/chat.json, webview-ui/src/i18n/locales/*/settings.json |
Adds unit tests verifying the compact-tool checkbox renders and toggles cached state; adds compactTool chat strings and contextManagement.compactToolUI settings translations across many locale files. |
Sequence Diagram
sequenceDiagram
participant User
participant SettingsView
participant ClineProvider
participant ExtensionStateContext
participant ChatRow
User->>SettingsView: toggle compactToolUI
SettingsView->>ClineProvider: set cached compactToolUI
ClineProvider->>ExtensionStateContext: post state including compactToolUI
ExtensionStateContext->>ChatRow: provide compactToolUI via context
ChatRow->>User: render compact single-line summary when enabled
User->>ChatRow: click to expand
ChatRow->>User: render full tool UI
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested reviewers
- taltas
- navedmerchant
- hannesrudolph
- JamesRobert20
Poem
🐰 I nibble code and tidy threads,
A chevron tucks away long spreads.
One slim line, a click to show—
Quiet chat, then details grow.
Hop, reveal, compact and neat.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately summarizes the main change: adding a compact tool UI mode for executed tool blocks in chat. |
| Linked Issues check | ✅ Passed | The implementation fully addresses issue #322: adds opt-in global setting, collapses executed (say) tool blocks to single line, keeps approval (ask) prompts expanded, and provides expansion interaction. |
| Out of Scope Changes check | ✅ Passed | All changes are directly scoped to the compact tool UI feature: schema/state additions, settings UI toggle, chat row rendering logic, internationalization keys, and related tests. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description check | ✅ Passed | The PR description comprehensively covers the linked issue, implementation details, scope, design decisions, and testing approach with clear explanations of the feature and its limitations. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
webview-ui/src/components/settings/SettingsView.tsx (1)
366-426:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
compactToolUIis not persisted on Save.Line 366 builds
updatedSettings, butcompactToolUIis missing, so the new toggle won’t survive save/reload.💡 Suggested fix
updatedSettings: { language, @@ showRooIgnoredFiles: showRooIgnoredFiles ?? true, + compactToolUI: compactToolUI ?? false, enableSubfolderRules: enableSubfolderRules ?? false,As per coding guidelines, “For
SettingsView, preserve the cached-state pattern: inputs should operate on localcachedStateuntil the user saves…”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/SettingsView.tsx` around lines 366 - 426, The updatedSettings object built in SettingsView omits compactToolUI so the toggle in cachedState never gets persisted; add compactToolUI: compactToolUI ?? /* default value or cachedState.compactToolUI */ to the updatedSettings payload (keeping the same null/undefined handling pattern as other fields) so the local cachedState value for compactToolUI is saved on Save and survives reload.
🧹 Nitpick comments (1)
webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx (1)
154-169: ⚡ Quick winAdd one SettingsView save-path regression test for
compactToolUI.These tests validate checkbox wiring, but not persistence through
updateSettings. A SettingsView test assertingvscode.postMessage({ type: "updateSettings", updatedSettings: { compactToolUI: ... } })would catch the current integration bug.As per coding guidelines, “Prefer local
webview-uitests for React/webview behavior such as component rendering, local state, hooks, form dirty-state, validation, or prop wiring.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx` around lines 154 - 169, Tests for ContextManagementSettings only check checkbox wiring but do not assert that toggling persists via SettingsView's updateSettings message; add a new test in the SettingsView test suite that mounts the SettingsView (or renders the component that wires ContextManagementSettings into the webview messaging layer), simulates toggling the compactToolUI checkbox, and asserts that vscode.postMessage was called with { type: "updateSettings", updatedSettings: { compactToolUI: true/false } }; locate the integration wiring around SettingsView (where ContextManagementSettings is used and updateSettings is invoked) and mock/spy on vscode.postMessage to validate the exact payload for compactToolUI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 1423-1434: Replace the clickable div used for compact-row
expansion with a semantic <button type="button"> so keyboard users can activate
it; move the onClick={handleToggleExpand} to the button, keep the className and
children (ChevronRight, PocketKnife, span), add an aria-expanded attribute bound
to the component's expanded state (e.g., aria-expanded={isExpanded} or the
appropriate state variable) and preserve data-testid="compact-tool-row"; ensure
the button does not submit forms (type="button") and that any focus/hover styles
remain intact so keyboard users can tab and press Enter/Space to toggle the
compact tool row.
---
Outside diff comments:
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 366-426: The updatedSettings object built in SettingsView omits
compactToolUI so the toggle in cachedState never gets persisted; add
compactToolUI: compactToolUI ?? /* default value or cachedState.compactToolUI */
to the updatedSettings payload (keeping the same null/undefined handling pattern
as other fields) so the local cachedState value for compactToolUI is saved on
Save and survives reload.
---
Nitpick comments:
In
`@webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx`:
- Around line 154-169: Tests for ContextManagementSettings only check checkbox
wiring but do not assert that toggling persists via SettingsView's
updateSettings message; add a new test in the SettingsView test suite that
mounts the SettingsView (or renders the component that wires
ContextManagementSettings into the webview messaging layer), simulates toggling
the compactToolUI checkbox, and asserts that vscode.postMessage was called with
{ type: "updateSettings", updatedSettings: { compactToolUI: true/false } };
locate the integration wiring around SettingsView (where
ContextManagementSettings is used and updateSettings is invoked) and mock/spy on
vscode.postMessage to validate the exact payload for compactToolUI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 24ee0f03-114c-47e8-8ef6-5328ecd340a5
📒 Files selected for processing (43)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…de-Org#322) Adds an opt-in global setting (compactToolUI, off by default) that collapses executed (say) tool blocks in the chat history to a single clickable line, hiding verbose descriptions and JSON payloads until the user expands them. Approval prompts (ask) are never compacted, so users always see the parameters they are approving. Toggle lives in Context Management settings; new i18n keys added across all 18 locales. Closes Zoo-Code-Org#322.
d5ae590 to
fd5ecdd
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@webview-ui/src/components/settings/SettingsView.tsx`:
- Around line 838-839: The save payload omits the compactToolUI field so
toggling it is not persisted; update the handleSubmit function to include
compactToolUI (the same state/prop used when rendering) in the updatedSettings
object before sending/saving, e.g. add compactToolUI: compactToolUI (or
compactToolUI ?? false) to updatedSettings so the toggle is persisted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 932d5575-a57a-49f6-b31c-0dc5f5e739cc
📒 Files selected for processing (43)
packages/types/src/global-settings.tspackages/types/src/vscode-extension-host.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/ContextManagementSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsxwebview-ui/src/context/ExtensionStateContext.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
✅ Files skipped from review due to trivial changes (21)
- webview-ui/src/i18n/locales/hi/chat.json
- webview-ui/src/i18n/locales/de/settings.json
- webview-ui/src/i18n/locales/es/chat.json
- webview-ui/src/i18n/locales/nl/settings.json
- webview-ui/src/i18n/locales/en/chat.json
- webview-ui/src/i18n/locales/hi/settings.json
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/ja/settings.json
- webview-ui/src/i18n/locales/fr/settings.json
- webview-ui/src/i18n/locales/tr/settings.json
- webview-ui/src/i18n/locales/zh-TW/chat.json
- webview-ui/src/i18n/locales/en/settings.json
- webview-ui/src/i18n/locales/ru/settings.json
- webview-ui/src/i18n/locales/ru/chat.json
- webview-ui/src/i18n/locales/ko/chat.json
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/ca/settings.json
- webview-ui/src/i18n/locales/fr/chat.json
- webview-ui/src/i18n/locales/zh-CN/settings.json
- webview-ui/src/i18n/locales/ja/chat.json
- webview-ui/src/i18n/locales/pt-BR/settings.json
- Add compactToolUI to updatedSettings in handleSubmit so the preference persists when saving - Change <div onClick> to <button> for keyboard accessibility and proper semantics
There was a problem hiding this comment.
🧹 Nitpick comments (1)
webview-ui/src/components/chat/ChatRow.tsx (1)
1423-1434: ⚡ Quick winAdd
aria-expandedattribute for screen reader support.The button correctly uses semantic HTML and supports keyboard interaction, but adding
aria-expanded={isExpanded}would help screen readers announce whether the compact tool row is currently expanded or collapsed.♿ Proposed accessibility enhancement
<button type="button" onClick={handleToggleExpand} + aria-expanded={isExpanded} className="flex items-center gap-2 py-0.5 cursor-pointer text-vscode-descriptionForeground hover:text-vscode-foreground bg-transparent border-none text-inherit w-full text-left" data-testid="compact-tool-row" title={t("chat:compactTool.expandHint")}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/chat/ChatRow.tsx` around lines 1423 - 1434, The compact tool toggle button in ChatRow.tsx lacks an aria-expanded attribute; update the button rendered alongside <ChevronRight /> and <PocketKnife /> (the element that calls handleToggleExpand and uses compactLabel) to include aria-expanded={isExpanded} so screen readers can detect expanded/collapsed state, keeping the existing props like data-testid and title unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 1423-1434: The compact tool toggle button in ChatRow.tsx lacks an
aria-expanded attribute; update the button rendered alongside <ChevronRight />
and <PocketKnife /> (the element that calls handleToggleExpand and uses
compactLabel) to include aria-expanded={isExpanded} so screen readers can detect
expanded/collapsed state, keeping the existing props like data-testid and title
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0afb8605-c6b1-4de5-8573-86a528b7da68
📒 Files selected for processing (2)
webview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/settings/SettingsView.tsx
getState() read compactToolUI from storage but never returned it, so getStateToPostToWebview() never sent the persisted value to the webview and the `false` default always won on reload. Wire it through both getState() returns following the showRooIgnoredFiles pattern. Addresses PR Zoo-Code-Org#327 review (edelauna).
|
Thanks for the review @edelauna!
|
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
Ok - lets just update the PR description, we can handle MCP history compaction in another issue/pr.
Had some minor questions.
| onClick={handleToggleExpand} | ||
| className="flex items-center gap-2 py-0.5 cursor-pointer text-vscode-descriptionForeground hover:text-vscode-foreground bg-transparent border-none text-inherit w-full text-left" | ||
| data-testid="compact-tool-row" | ||
| title={t("chat:compactTool.expandHint")}> |
There was a problem hiding this comment.
Would adding aria-expanded={false} here round out the a11y fix? Since this button only renders in the collapsed state, the value is always false, but it signals to screen readers that this is an expandable control.
| title={t("chat:compactTool.expandHint")}> | |
| title={t("chat:compactTool.expandHint")} | |
| aria-expanded={false}> |
There was a problem hiding this comment.
Good call — added aria-expanded={false} on the compact toggle button. Since it only renders in the collapsed state the value is always false, so it just signals to screen readers that this is an expandable control. It's on the <button> at ChatRow.tsx:1426.
| await waitFor(() => { | ||
| expect(setCachedStateField).toHaveBeenCalledWith("compactToolUI", true) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These cover the settings toggle nicely. Would it be worth also adding a test that mounts ChatRowContent with compactToolUI=true and a say tool message, to verify the compact row renders (and that clicking it calls onToggleExpand)? That would give a regression guard on the actual chat-side behaviour.
There was a problem hiding this comment.
Done — added webview-ui/src/components/chat/__tests__/ChatRow.compact-tool.spec.tsx. It mounts ChatRowContent with compactToolUI=true and a say tool message and asserts: (1) the compact row renders, (2) clicking it calls onToggleExpand(message.ts), and (3) the button carries aria-expanded="false". Good regression guard — thanks for the nudge.
|
Conflicts resolved and branch rebased on latest main. CI is passing. Ready for re-review. 🙏 |
Addresses CodeRabbit review nitpick: add aria-expanded={isExpanded}
to the compact tool row button for screen reader support.
Refs: PR Zoo-Code-Org#327 review feedback
|
Thanks for the review @edelauna! I've addressed the feedback: Changes made:
Regarding your comments:
The CI checks should pick up the new commit shortly. Let me know if there are any other questions or changes needed! |
…dd ChatRow compact tool tests
- Change aria-expanded={isExpanded} to aria-expanded={false} per edelauna review
(button only renders when !isExpanded, so value is always false)
- Add ChatRow.compact-tool.spec.tsx with 3 tests:
1. Renders compact row when compactToolUI=true and not expanded
2. Calls onToggleExpand when compact row is clicked
3. Has aria-expanded=false on the compact button
Addresses review feedback on PR Zoo-Code-Org#327
|
Updated per your review @edelauna:
All tests pass. Let me know if you'd like any further changes! |
|
@edelauna addressed your notes:
Persistence across reloads is wired through |
|
@edelauna — friendly nudge for a re-review when you have a moment 🙏. Everything from the last round is in:
Branch is MERGEABLE. Thanks! |
|
@edelauna — friendly nudge for re-review 🙏 All feedback from the last round has been addressed:
Let me know if you need any further changes! |
| <SearchableSetting | ||
| settingId="context-compact-tool-ui" | ||
| section="contextManagement" | ||
| label={t("settings:contextManagement.compactToolUI.label")}> | ||
| <VSCodeCheckbox | ||
| checked={compactToolUI} | ||
| onChange={(e: any) => setCachedStateField("compactToolUI", e.target.checked)} | ||
| data-testid="compact-tool-ui-checkbox"> | ||
| <label className="block font-medium mb-1"> | ||
| {t("settings:contextManagement.compactToolUI.label")} | ||
| </label> | ||
| </VSCodeCheckbox> | ||
| <div className="text-vscode-descriptionForeground text-sm mt-1 mb-3"> | ||
| {t("settings:contextManagement.compactToolUI.description")} | ||
| </div> | ||
| </SearchableSetting> |
There was a problem hiding this comment.
Should this live in UISettings.tsx instead? compactToolUI controls how chat rows render (display preference), while this section manages what data gets sent to the model's context window (maxOpenTabsContext, maxWorkspaceFiles). UISettings already has the analogous reasoningBlockCollapsed (collapse thinking blocks in chat).
| describe("ChatRow - compact tool UI", () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| mockOnToggleExpand.mockClear() | ||
| }) | ||
|
|
||
| it("renders the compact row when compactToolUI is true and not expanded", () => { | ||
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | ||
| renderChatRow(message, false) | ||
|
|
||
| expect(screen.getByTestId("compact-tool-row")).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it("calls onToggleExpand when the compact row is clicked", () => { | ||
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | ||
| renderChatRow(message, false) | ||
|
|
||
| fireEvent.click(screen.getByTestId("compact-tool-row")) | ||
|
|
||
| expect(mockOnToggleExpand).toHaveBeenCalledWith(message.ts) | ||
| }) | ||
|
|
||
| it("has aria-expanded=false on the compact button", () => { | ||
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | ||
| renderChatRow(message, false) | ||
|
|
||
| expect(screen.getByTestId("compact-tool-row")).toHaveAttribute("aria-expanded", "false") | ||
| }) | ||
| }) |
There was a problem hiding this comment.
These cover the happy path well. Worth adding negative-path cases for isExpanded=true and compactToolUI=false? Both branches guard the compact render — if either condition is accidentally dropped, no test would catch it.
| describe("ChatRow - compact tool UI", () => { | |
| beforeEach(() => { | |
| vi.clearAllMocks() | |
| mockOnToggleExpand.mockClear() | |
| }) | |
| it("renders the compact row when compactToolUI is true and not expanded", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| expect(screen.getByTestId("compact-tool-row")).toBeInTheDocument() | |
| }) | |
| it("calls onToggleExpand when the compact row is clicked", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| fireEvent.click(screen.getByTestId("compact-tool-row")) | |
| expect(mockOnToggleExpand).toHaveBeenCalledWith(message.ts) | |
| }) | |
| it("has aria-expanded=false on the compact button", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| expect(screen.getByTestId("compact-tool-row")).toHaveAttribute("aria-expanded", "false") | |
| }) | |
| }) | |
| describe("ChatRow - compact tool UI", () => { | |
| beforeEach(() => { | |
| vi.clearAllMocks() | |
| mockOnToggleExpand.mockClear() | |
| }) | |
| it("renders the compact row when compactToolUI is true and not expanded", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| expect(screen.getByTestId("compact-tool-row")).toBeInTheDocument() | |
| }) | |
| it("does NOT render compact row when isExpanded is true", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, true) | |
| expect(screen.queryByTestId("compact-tool-row")).not.toBeInTheDocument() | |
| }) | |
| it("calls onToggleExpand when the compact row is clicked", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| fireEvent.click(screen.getByTestId("compact-tool-row")) | |
| expect(mockOnToggleExpand).toHaveBeenCalledWith(message.ts) | |
| }) | |
| it("has aria-expanded=false on the compact button", () => { | |
| const message = createSayToolMessage({ tool: "readFile", path: "src/file.ts" }) | |
| renderChatRow(message, false) | |
| expect(screen.getByTestId("compact-tool-row")).toHaveAttribute("aria-expanded", "false") | |
| }) | |
| }) |
|
@proyectoauraorg You have no idea how grateful I am to you! It was worth researching pulls. This is probably the most useful modification you've made. I was going to implement all of this from scratch. And there should be three modes. 1) Normal, like now.
Moreover, it should be flexibly configured for each type (MCP, terminal commands, file operations. I want to implement all this in my own place. |
edelauna
left a comment
edelauna
left a comment
There was a problem hiding this comment.
@proyectoauraorg can you add some screenshots for how the compaction works? I'm not able to reproduce it, and see a difference between before or after.
github-actions
Bot
added
has-conflicts
3 participants
Related GitHub Issue
Closes #322
Description
Adds an opt-in Compact tool display setting (
compactToolUI, off by default) that collapses executed (say) tool blocks in the chat history to a single, clickable line — hiding long descriptions and JSON payloads until the user clicks to expand. This addresses the issue's complaint: verbose tool blocks push the actual conversation out of view and make history tedious to scroll.Scope
Compaction applies to executed file-operation tool rows — the
say→case "tool"branch inChatRow. Theuse_mcp_serverbranch (line ~1623) is intentionally left untouched in this PR. Per review discussion, MCP history compaction will be handled in a separate follow-up issue/PR, since MCP blocks have a different shape (server/tool/arguments/response) and deserve their own compact layout.Design decision: approval prompts (
asktool messages) are never compacted — only completed/history (say) tool rows are — so users always see the parameters they are approving before acting.How it works
compactToolUIinglobalSettingsSchema+ExtensionState, persisted viaClineProvider.getState()/getStateToPostToWebview()so it survives reloads.compact-tool-ui-checkbox).ChatRow, thesaytool branch renders a single line (chevron + 🔧 + Tool: <name/path>) whencompactToolUI && !isExpanded; clicking the row (a semantic<button>witharia-expanded={false}) toggles expansion and falls through to the normal full render.settings:contextManagement.compactToolUI.*,chat:compactTool.*) added across all 18 locales.Testing
ContextManagementSettings.spec.tsx: toggle renders + firessetCachedStateField("compactToolUI", …).ChatRow.compact-tool.spec.tsx: mountsChatRowContentwithcompactToolUI=trueand asaytool message; asserts the compact row renders, that clicking it callsonToggleExpand(message.ts), and that the button carriesaria-expanded="false".tsc -b(webview) andtsc --noEmit(src) clean; eslint clean;find-missing-translationsreports complete parity.